0.5 Month Internship Review

2019-01-25

0.5 months, 2 weeks, handled a total of 2 work orders.

Work Order 1

The content of the first work order was relatively simple. The requirement was to change the existing behavior where a certain field being empty did not allow duplicate entries, to allow duplicates. The solution was also simple: skip duplicate checks when the field is empty. The primary focus at the beginning was to familiarize myself with the development process.

  1. Confirm the requirement and create a work order in the OA system;
  2. Create a new branch and start development;
  3. Self-test;
  4. Code review;
  5. Submit for testing;
  6. Receive the test report and request deployment;
  7. Complete deployment and merge the code into the baseline.

In fact, communication incurs certain costs, and more complex requirements may have higher communication costs.

Work Order 2

The second work order was to fix platform vulnerabilities, including one SQL injection vulnerability and one reflected XSS attack vulnerability. The solution approach was also conventional.

The first issue corrected after the code review, and also my first lesson, was that if-else statements should not be nested too deeply in the program. Fixing the SQL injection vulnerability required checking multiple parameters. Initially, I did it like this:

if (param1) {

} else if (param2) {

} else {

}

This approach is terrible! Moreover, it is clearly not recommended by coding standards. After my mentor’s reminder, I changed the code to a more extensible approach:

Map<String, String> paramMap = new ConcurrentHashMap<>();
map.put(param1);
map.put(param2);
if (map) {}

At first glance, there seems to be no problem, but this code has a lurking bug. The second lesson I learned is that ConcurrentHashMap does not allow null values! Compared to HashMap, it has a significant NPE risk because the parameters passed here come from the page (or should we validate for null when passing parameters?). Production environment code cannot afford mistakes.

The third lesson is about configuration files. The configuration file of the program should achieve the effect that without these lines of configuration, the program should behave like an offline version, with the changes or additions dependent on this configuration file. Of course, this also requires that the program handles the absence of configurations appropriately. Naturally, I did not consider in advance that if the configuration is missing, a direct NPE would occur.

The fourth lesson is about over-engineering. The program went live the night before, and I received an exception report at 8 AM the next day. After starting work, I immediately rolled back. The exception was caused by me modifying parts of the code outside the requirements. There were two identical methods, one above had parameter validation added, and although it wasn’t specified in the requirement, I went ahead and added parameter validation to the method below as well. Then, NPE occurred. Since it was outside the requirement, self-testing didn’t catch it, code review didn’t catch it, and testing didn’t catch it, but it emerged after going live.

Rolling back the program caused a loss roughly equating to the online system having exceptions for one night, with the involved personnel having to go through the work order process again to fix the bug. Just for two lines of code.

Summary

  1. Be responsible for the code, from requirement to deployment.
  2. Actively handle other matters, respond quickly, communicate smoothly, and stay focused.